-
Notifications
You must be signed in to change notification settings - Fork 14.4k
rtsan: Support free_sized and free_aligned_sized from C23 #145085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Justin King (jcking) ChangesAdds support to RTSan for Other sanitizers will be handled with their own separate PRs. For #144435 Full diff: https://github.com/llvm/llvm-project/pull/145085.diff 3 Files Affected:
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 9f9ffd4313810..a9d864e9fe926 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -869,6 +869,48 @@ INTERCEPTOR(void, free, void *ptr) {
return REAL(free)(ptr);
}
+#if SANITIZER_INTERCEPT_FREE_SIZED
+INTERCEPTOR(void, free_sized, void *ptr, SIZE_T size) {
+ if (DlsymAlloc::PointerIsMine(ptr))
+ return DlsymAlloc::Free(ptr);
+
+ // According to the C and C++ standard, freeing a nullptr is guaranteed to be
+ // a no-op (and thus real-time safe). This can be confirmed for looking at
+ // __libc_free in the glibc source.
+ if (ptr != nullptr)
+ __rtsan_notify_intercepted_call("free_sized");
+
+ if (REAL(free_sized))
+ return REAL(free_sized)(ptr, size);
+ return REAL(free)(ptr);
+}
+#define RTSAN_MAYBE_INTERCEPT_FREE_SIZED INTERCEPT_FUNCTION(free_sized)
+#else
+#define RTSAN_MAYBE_INTERCEPT_FREE_SIZED
+#endif
+
+#if SANITIZER_INTERCEPT_FREE_ALIGNED_SIZED
+INTERCEPTOR(void, free_aligned_sized, void *ptr, SIZE_T alignment,
+ SIZE_T size) {
+ if (DlsymAlloc::PointerIsMine(ptr))
+ return DlsymAlloc::Free(ptr);
+
+ // According to the C and C++ standard, freeing a nullptr is guaranteed to be
+ // a no-op (and thus real-time safe). This can be confirmed for looking at
+ // __libc_free in the glibc source.
+ if (ptr != nullptr)
+ __rtsan_notify_intercepted_call("free_aligned_sized");
+
+ if (REAL(free_aligned_sized))
+ return REAL(free_aligned_sized)(ptr, alignment, size);
+ return REAL(free)(ptr);
+}
+#define RTSAN_MAYBE_INTERCEPT_FREE_ALIGNED_SIZED \
+ INTERCEPT_FUNCTION(free_aligned_sized)
+#else
+#define RTSAN_MAYBE_INTERCEPT_FREE_ALIGNED_SIZED
+#endif
+
INTERCEPTOR(void *, malloc, SIZE_T size) {
if (DlsymAlloc::Use())
return DlsymAlloc::Allocate(size);
@@ -1493,6 +1535,8 @@ INTERCEPTOR(INT_TYPE_SYSCALL, syscall, INT_TYPE_SYSCALL number, ...) {
void __rtsan::InitializeInterceptors() {
INTERCEPT_FUNCTION(calloc);
INTERCEPT_FUNCTION(free);
+ RTSAN_MAYBE_INTERCEPT_FREE_SIZED;
+ RTSAN_MAYBE_INTERCEPT_FREE_ALIGNED_SIZED;
INTERCEPT_FUNCTION(malloc);
INTERCEPT_FUNCTION(realloc);
INTERCEPT_FUNCTION(reallocf);
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/free_aligned_sized.c b/compiler-rt/test/sanitizer_common/TestCases/Linux/free_aligned_sized.c
index 7710c62368191..42a68b76db902 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/free_aligned_sized.c
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/free_aligned_sized.c
@@ -1,13 +1,25 @@
// RUN: %clang -std=c23 -O0 %s -o %t && %run %t
-// UNSUPPORTED: asan, hwasan, rtsan, tsan, ubsan
+// UNSUPPORTED: asan, hwasan, tsan, ubsan
#include <stddef.h>
#include <stdlib.h>
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+# include <sanitizer/rtsan_interface.h>
+#endif
+
+extern void *aligned_alloc(size_t alignment, size_t size);
+
extern void free_aligned_sized(void *p, size_t alignment, size_t size);
int main() {
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+ __rtsan_disable();
+#endif
volatile void *p = aligned_alloc(128, 1024);
free_aligned_sized((void *)p, 128, 1024);
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+ __rtsan_enable();
+#endif
return 0;
}
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/free_sized.c b/compiler-rt/test/sanitizer_common/TestCases/Linux/free_sized.c
index 9eac562fecb03..8453bcf07974a 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/free_sized.c
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/free_sized.c
@@ -1,15 +1,23 @@
// RUN: %clang -std=c23 -O0 %s -o %t && %run %t
-// UNSUPPORTED: asan, hwasan, rtsan, tsan, ubsan
+// UNSUPPORTED: asan, hwasan, tsan, ubsan
#include <stddef.h>
#include <stdlib.h>
-extern void *aligned_alloc(size_t alignment, size_t size);
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+# include <sanitizer/rtsan_interface.h>
+#endif
extern void free_sized(void *p, size_t size);
int main() {
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+ __rtsan_disable();
+#endif
volatile void *p = malloc(64);
free_sized((void *)p, 64);
+#if defined(__has_feature) && __has_feature(realtime_sanitizer)
+ __rtsan_enable();
+#endif
return 0;
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few questions, otherwise looks good.
Would you add a test to rtsan_test_interceptors_posix.cpp too?
You can copy and paste an existing one, but it would be good to have:
- each new function with a valid pointer
- each new function with an invalid pointer (doesn't die when instrumented)
Thanks for implementing these
if (ptr != nullptr) | ||
__rtsan_notify_intercepted_call("free_sized"); | ||
|
||
if (REAL(free_sized)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand what the conditions would have to be to be in this interceptor, but not have the REAL free_sized
available?
It seems like to be in this interceptor, the real one would have to exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On ELF-based systems, like Linux, the interception code doesn't require that the underlying real function actually exist. This will result in the real function pointer being nullptr
. In that case, we forward to free
which we assume exists or we have bigger problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we should do with the rest of our interceptors as well?
It was my previous understanding that if someone got into this interceptor without a valid REAL version on their system, their binary would likely crash and die (which is acceptable, because they called some illegal function).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not. In this case, an acceptable implementation of free_sized
and free_aligned_sized
is just calling free
. Since we have that option easily implementable here, we forward. The likelihood that somebody manages to call this function without a real underlying function is low, but it still seemed better to not crash if somebody managed to dlsym
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks
compiler-rt/test/sanitizer_common/TestCases/Linux/free_aligned_sized.c
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin King <[email protected]>
84b991e
to
46c2648
Compare
Added tests. Let me know if what I added was along the lines of what you were expecting. |
looks perfect, thanks! |
Adds support to RTSan for
free_sized
andfree_aligned_sized
from C23.Other sanitizers will be handled with their own separate PRs.
For #144435